feat: add dark mode support - #222
phillip9933 wants to merge 20 commits into
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
The new Appearance preference handling has a correctness issue for missing/invalid persisted values, and there are dark-mode theming gaps (e.g., selection and dialogs) that should be addressed before merging.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR introduces app-wide dark mode support by switching to a DayNight base theme and migrating many UI surfaces/text to theme-resolved semantic colors, plus adds a user-facing appearance mode setting.
Changes:
- Switch
Theme.openCloudtoTheme.AppCompat.DayNight.*and introduce semantic color tokens withvalues-nightoverrides. - Replace hardcoded light colors in many layouts/drawables with
?attr/*/?android:attr/*theme attributes. - Add an “Appearance” setting backed by an
AppearanceModeenum and apply it at app startup viaAppCompatDelegate.
File summaries
| File | Description |
|---|---|
| opencloudApp/src/test/java/eu/opencloud/android/presentation/settings/AppearanceModeTest.kt | Adds unit tests for preference-to-night-mode mapping. |
| opencloudApp/src/main/res/xml/settings.xml | Adds the Appearance ListPreference to settings screen. |
| opencloudApp/src/main/res/values/styles.xml | Switches base theme to DayNight; adds semantic theme color items. |
| opencloudApp/src/main/res/values/strings.xml | Adds Appearance preference strings. |
| opencloudApp/src/main/res/values/colors.xml | Adds DayNight semantic color tokens (light defaults). |
| opencloudApp/src/main/res/values-night/colors.xml | Adds DayNight semantic color tokens (dark overrides). |
| opencloudApp/src/main/res/layout/whats_new_element.xml | Updates text colors to theme-aware colorOnPrimary. |
| opencloudApp/src/main/res/layout/whats_new_activity.xml | Updates skip button text color to theme-aware colorOnPrimary. |
| opencloudApp/src/main/res/layout/uploader_list_item_layout.xml | Migrates uploader list text colors to colorOnSurface*. |
| opencloudApp/src/main/res/layout/uploader_layout.xml | Migrates list divider color to theme colorOutline. |
| opencloudApp/src/main/res/layout/upload_list_item.xml | Migrates upload list text colors to colorOnSurface*. |
| opencloudApp/src/main/res/layout/upload_bottom_sheet_fragment.xml | Migrates bottom sheet background/divider/title colors to theme. |
| opencloudApp/src/main/res/layout/ssl_validator_layout.xml | Migrates SSL validator text color to colorOnSurface. |
| opencloudApp/src/main/res/layout/ssl_untrusted_cert_layout.xml | Migrates SSL untrusted cert header color to colorOnSurface. |
| opencloudApp/src/main/res/layout/spaces_list_item.xml | Migrates card background and text colors to surface/on-surface. |
| opencloudApp/src/main/res/layout/space_path_line.xml | Migrates icon/text tint to colorOnSurfaceVariant. |
| opencloudApp/src/main/res/layout/space_header.xml | Migrates header backgrounds and text colors to surface/on-surface. |
| opencloudApp/src/main/res/layout/sort_options_layout.xml | Migrates bottom sheet header text/icon tint to colorOnPrimary. |
| opencloudApp/src/main/res/layout/sort_bottom_sheet_fragment.xml | Migrates bottom sheet background/divider/title colors to theme. |
| opencloudApp/src/main/res/layout/share_user_item.xml | Migrates share user item text color to colorOnSurface. |
| opencloudApp/src/main/res/layout/share_public_link_item.xml | Migrates public link item title color to colorOnSurface. |
| opencloudApp/src/main/res/layout/share_public_dialog.xml | Migrates dialog label/secondary text colors to theme-aware attrs. |
| opencloudApp/src/main/res/layout/share_file_layout.xml | Migrates share file sheet colors to on-surface/on-primary. |
| opencloudApp/src/main/res/layout/remove_files_dialog.xml | Migrates dialog card/text colors to surface/on-surface. |
| opencloudApp/src/main/res/layout/rate_me_dialog.xml | Migrates button text to colorOnPrimary. |
| opencloudApp/src/main/res/layout/passcodelock.xml | Migrates passcode lock backgrounds/text to theme attributes. |
| opencloudApp/src/main/res/layout/opencloud_toolbar.xml | Migrates toolbar text/drawable tint to colorOnPrimary. |
| opencloudApp/src/main/res/layout/newfile_bottom_sheet_fragment.xml | Migrates bottom sheet background/divider/title colors to theme. |
| opencloudApp/src/main/res/layout/nav_main_container.xml | Migrates main container background to platform background attr. |
| opencloudApp/src/main/res/layout/nav_drawer_header.xml | Migrates drawer header text color to colorOnPrimary. |
| opencloudApp/src/main/res/layout/nav_drawer_footer.xml | Migrates drawer footer text colors to on-surface variants. |
| opencloudApp/src/main/res/layout/nav_drawer_content.xml | Sets drawer backgrounds to theme colorSurface. |
| opencloudApp/src/main/res/layout/nav_coordinator_layout.xml | Migrates bottom nav icon/text tint to colorOnPrimary. |
| opencloudApp/src/main/res/layout/manage_accounts_dialog.xml | Migrates dialog card/text/icon tint to theme attributes. |
| opencloudApp/src/main/res/layout/main_file_list_fragment.xml | Migrates FAB label/icon colors to colorOnPrimary. |
| opencloudApp/src/main/res/layout/log_list_item.xml | Migrates log list text colors to on-surface variants. |
| opencloudApp/src/main/res/layout/list_footer.xml | Migrates footer text color to colorOnSurfaceVariant. |
| opencloudApp/src/main/res/layout/item_file_list.xml | Adds surface background + migrates text/tints to on-surface variants. |
| opencloudApp/src/main/res/layout/item_empty_dataset.xml | Migrates empty state tint/text colors to theme attributes. |
| opencloudApp/src/main/res/layout/grid_item.xml | Migrates grid item text color to colorOnSurface. |
| opencloudApp/src/main/res/layout/fragment_migration_progress.xml | Migrates wizard background/text/progress tint to primary/on-primary. |
| opencloudApp/src/main/res/layout/fragment_migration_intro.xml | Migrates wizard background/text/button tint to primary/on-primary. |
| opencloudApp/src/main/res/layout/fragment_migration_completed.xml | Migrates wizard background/text/button tint to primary/on-primary. |
| opencloudApp/src/main/res/layout/fragment_migration_choice.xml | Migrates wizard background/text/button tint to primary/on-primary. |
| opencloudApp/src/main/res/layout/file_options_bottom_sheet_fragment.xml | Migrates bottom sheet background/text/divider to theme attrs. |
| opencloudApp/src/main/res/layout/file_details_fragment.xml | Migrates card/background/dividers to surface/outline theme attrs. |
| opencloudApp/src/main/res/layout/file_details_empty.xml | Migrates empty background to platform background attr. |
| opencloudApp/src/main/res/layout/edit_share_layout.xml | Migrates secondary text color to colorOnSurfaceVariant. |
| opencloudApp/src/main/res/layout/dialog_file_already_exists.xml | Migrates dialog card/text colors to surface/on-surface. |
| opencloudApp/src/main/res/layout/create_shortcut_dialog.xml | Migrates labels/disabled button text to on-surface variants. |
| opencloudApp/src/main/res/layout/activity_pattern_lock.xml | Migrates background/text colors to theme attributes. |
| opencloudApp/src/main/res/layout/account_item.xml | Migrates account item text/icon tints to on-surface variants. |
| opencloudApp/src/main/res/layout/account_action.xml | Migrates account action icon/text to on-surface colors. |
| opencloudApp/src/main/res/drawable/top_progress_bar_determinate.xml | Makes progress bar background use theme colorSurface. |
| opencloudApp/src/main/java/eu/opencloud/android/presentation/settings/SettingsFragment.kt | Wires up Appearance preference and applies night mode on change. |
| opencloudApp/src/main/java/eu/opencloud/android/presentation/settings/AppearanceMode.kt | Introduces AppearanceMode enum + preference mapping helper. |
| opencloudApp/src/main/java/eu/opencloud/android/presentation/files/filelist/FileListAdapter.kt | Replaces hardcoded white with theme colorSurface for file list items. |
| opencloudApp/src/main/java/eu/opencloud/android/MainApp.kt | Applies persisted appearance mode at application startup. |
| opencloudApp/src/androidTest/java/eu/opencloud/android/settings/SettingsFragmentTest.kt | Extends UI test to validate Appearance preference configuration. |
Review details
- Files reviewed: 59/59 changed files
- Comments generated: 3
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
thanks for the sanity check with co-pilot. I've updated the issues it called out, should be ready for re-review |
There was a problem hiding this comment.
🟢 Approval recommended
Only a minor maintainability nit was found (redundant background assignment in FileListAdapter), with no functional or test issues identified.
Review details
- Files reviewed: 63/63 changed files
- Comments generated: 1
- Review effort level: Lite
| holder.itemView.setBackgroundColor(colorSurface) | ||
|
|
||
| val checkBoxV = holder.itemView.findViewById<ImageView>(R.id.custom_checkbox).apply { | ||
| isVisible = hasActiveSelection | ||
| } | ||
|
|
||
| if (isSelected(position)) { | ||
| holder.itemView.setBackgroundColor(ContextCompat.getColor(context, R.color.selected_item_background)) | ||
| holder.itemView.setBackgroundColor(colorSelection) | ||
| checkBoxV.setImageResource(R.drawable.ic_checkbox_marked) | ||
| } else { | ||
| holder.itemView.setBackgroundColor(Color.WHITE) | ||
| holder.itemView.setBackgroundColor(colorSurface) | ||
| checkBoxV.setImageResource(R.drawable.ic_checkbox_blank_outline) |
wernrab
left a comment
There was a problem hiding this comment.
Looks ok, I also have tested it briefly.
|
@phillip9933 thank you for your pull request! We're currently reviewing it. One thing: Detekt is reporting a missing newline at the end of some files - could you please fix that? 😇 |
There was a problem hiding this comment.
🟡 Changes recommended
One or more issues must be addressed before approval.
Get a fresh assessment by requesting another Copilot review.
Review details
Suppressed comments (9)
opencloudApp/src/main/java/eu/opencloud/android/MainApp.kt:114
setDefaultNightModeonly updates activities managed by anAppCompatDelegate, butWhatsNewActivityis aFragmentActivity(andCopyToClipboardActivityis a frameworkActivity). With the system set to a different mode, those activities do not receive this user-selected Light/Dark mode. Migrate the remaining framework activities to AppCompat or apply the stored mode/configuration to them explicitly.
AppCompatDelegate.setDefaultNightMode(appearanceMode.nightMode)
opencloudApp/src/main/java/eu/opencloud/android/presentation/accounts/ManageAccountsDialogFragment.kt:81
AlertDialoghere is stillandroid.app.AlertDialog, so removing the explicitTheme_AppCompat_Dialog_Alertwrapper means this builder does not consume the AppCompatalertDialogThemeconfigured inTheme.openCloud. The manage-accounts dialog can consequently use platform/light dialog chrome instead of the selected DayNight theme; use the AppCompat builder or retain the wrapper.
val builder = AlertDialog.Builder(requireContext())
opencloudApp/src/main/java/eu/opencloud/android/presentation/files/filelist/FileListAdapter.kt:276
- This row color is themed, but the PNG branch below still sets the thumbnail background to
R.color.background_color(line 313). Transparent PNG thumbnails will therefore retain a white background in dark mode; use the themed surface color for that branch as well.
holder.itemView.setBackgroundColor(colorSurface)
opencloudApp/src/main/java/eu/opencloud/android/presentation/settings/SettingsFragment.kt:78
- The new tests verify the enum mapping and preference initialization, but none exercises this change listener. A regression here could leave the preference value changing without calling
AppCompatDelegate.setDefaultNightMode, so add an instrumentation test that selects LIGHT/DARK and verifies the applied night mode (and persisted value).
setOnPreferenceChangeListener { preference, newValue ->
val appearanceMode = AppearanceMode.fromPreferenceValue(newValue as String)
preference.summary = entries[findIndexOfValue(appearanceMode.name)]
AppCompatDelegate.setDefaultNightMode(appearanceMode.nightMode)
true
opencloudApp/src/main/res/drawable/numberkeyboard_key_bg_pressed.xml:4
- The new night pressed background is
#31515D, but every keypad label still usesnumberkeyboard_key_text_color.xml, whose pressed-state color is the fixed@color/color_accent(#396676). The pressed labels therefore have very low contrast in Dark mode. Make the pressed text color theme-aware and choose a color with sufficient contrast againstcolorSelection.
<solid android:color="?attr/colorSelection" />
opencloudApp/src/main/res/layout/edit_share_layout.xml:108
- The two visible sharing switch labels above still hardcode
@color/color_accent(lines 63 and 95). In the new DayNight theme they sit on the dark window background, where#396676has insufficient text contrast; changing only the error label tocolorOnSurfaceVariantleaves the primary controls hard to read. Use a light surface text token for these labels or provide a night color.
android:textColor="?attr/colorOnSurfaceVariant"
opencloudApp/src/main/res/layout/rate_me_dialog.xml:70
- The buttons now use
colorOnPrimary, but the same dark-themed dialog still renders its title and description with the unchanged hardcoded@color/color_accent(#396676) at lines 34 and 50. Once the DayNight dialog surface is#1E1E1E, those 20sp/16sp texts have insufficient contrast. Make these dialog text colors use a light semantic surface color or provide a night override as well.
opencloudApp/src/main/res/layout/remove_files_dialog.xml:14 - This card is now
colorSurfacein night mode, but itsButton.Borderlesschildren still get the shared hardcoded@color/color_accenttext color (#396676). On the night surface (#1E1E1E) that is low contrast, so the dialog actions become difficult to read. Make the shared borderless-button text color theme-aware (and update the other custom dialogs that use it).
opencloudApp/src/main/res/values/styles.xml:101 - Switching the alert dialog to a DayNight parent makes its surface dark, but
buttonBarButtonStylestill inheritsWidget.AppCompat.Button.Borderless.Colored, whose action text usescolorAccent(#396676). That color has insufficient contrast on the night surface (#1E1E1E), so the positive/negative actions of the many themed alert dialogs become difficult to read. Give the dialog button style a night-aware/light semantic text color instead of relying on the accent.
- Files reviewed: 63/63 changed files
- Comments generated: 4
- Review effort level: Lite
| android:layout_width="match_parent" | ||
| android:layout_height="0dp" | ||
| android:background="@color/background_color" | ||
| android:background="?android:attr/colorBackground" |
| android:layout_marginStart="@dimen/standard_half_margin" | ||
| android:singleLine="true" | ||
| android:textColor="@color/textColor" | ||
| android:textColor="?attr/colorOnSurface" |
| android:layout_width="match_parent" | ||
| android:layout_height="0dp" | ||
| android:background="@color/background_color" | ||
| android:background="?android:attr/colorBackground" |
| android:layout_width="match_parent" | ||
| android:layout_height="wrap_content" | ||
| android:background="@color/white" | ||
| android:background="?attr/colorSurface" |
| <ListPreference | ||
| app:defaultValue="SYSTEM" | ||
| app:dialogTitle="@string/prefs_appearance" | ||
| app:iconSpaceReserved="false" |
There was a problem hiding this comment.
Shall we left-align all items to each other? Or even use an icon here as for all other settings?
There was a problem hiding this comment.
Ah good call I think using an icon there is probably the better option so things are more consistent in settings. I'll take a look at that ina bit. (JP timezone).
I also saw the new Copilot feedback. A few of those look like legitimate leftovers from the dark mode work, particularly the non-AppCompat activities and dialogs and a few places that are still relying on the old accent/bg colors. Instead of
patching each one by itself I'll see if I can clean those up using the semantic theme colors so Light/Dark/System behave consistently.
I also saw the earlier Detekt comments about the missing newlines earlier and I'll get those cleaned up as well.
I'm just getting back from vacation so it'll probably be a day or so before I can think through that part and get it fixed.
Also for what its worth as a bit of a side project I've also been experimenting with a native Kotlin/Jetpack Compose rewrite of the android app. I was struggling quite a bit working through some of the existing Java implementation with this change and Kotlin/Compose is much closer to what I'm comfortable working with. I've also been gradually de-Googling my own setup and since I'm recently semi-retired I've had some extra time to experiment with what a more oCIS/Libre Graph-native client could look like. very much a personal experiment currently though and not intended to overshadow this PR or the work that's already gone into the existing app. I'll get back to the changes here tomorrow.
Summary
Adds dark mode support by replacing hardcoded white backgrounds with theme-aware
DayNightattributes across the app.Closes #23
Changes
styles.xml) to use platform semantic background attributes linked totheme_color_background.FileListAdapteranditem_file_list.xmlto use?attr/colorSurfaceinstead of hardcoded white backgrounds.?android:attr/colorBackground.Testing
./gradlew :opencloudApp:assembleOriginalDebug.